Skip to content

fix: error suppressing tests on packages#4996

Merged
auto-submit[bot] merged 5 commits intomainfrom
suppressPackages
Mar 24, 2026
Merged

fix: error suppressing tests on packages#4996
auto-submit[bot] merged 5 commits intomainfrom
suppressPackages

Conversation

@jtmcdole
Copy link
Member

various problems:

  • dialog widgets aren't mounted and getting dismissed
  • githubService.getIssue() is odd (Future?)
  • Add fake auth to dashboard for non-prod
  • Add fake issue response for testing
  • see flutter/packages

various problems:
- dialog widgets aren't mounted and getting dismissed
- githubService.getIssue() is odd (Future<Issue>?)
- Add fake auth to dashboard for non-prod
- Add fake issue response for testing
- see flutter/packages
@github-actions
Copy link
Contributor

🤖 Hi @jtmcdole, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

@jtmcdole jtmcdole added the CICD Run CI/CD label Mar 24, 2026
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

## 📋 Review Summary

This PR addresses multiple issues across the project, including handling missing dialog widget mounting, enhancing the fake auth integration in the dashboard, and extending test data seeding logic. Overall, the implementation correctly resolves the issues and adds appropriate error handling and test coverage.

🔍 General Feedback

  • Good additions of mock behavior (fakeGetIssue) and tests to ensure the changes are adequately covered.
  • The use of the try...catch block in UpdateSuppressedTest is a good addition for robustness but should avoid catching and masking expected exceptions.
  • Be careful with Dart map syntax for conditionally including elements, especially when migrating to newer patterns.
  • Excellent job on providing fake implementations (FakeFirebaseAuthService) for non-production environments to streamline development and testing.

@jtmcdole jtmcdole added CICD Run CI/CD and removed CICD Run CI/CD labels Mar 24, 2026
@jtmcdole jtmcdole requested a review from ievdokdm March 24, 2026 18:51
@jtmcdole jtmcdole added CICD Run CI/CD and removed CICD Run CI/CD labels Mar 24, 2026
@jtmcdole jtmcdole requested a review from ievdokdm March 24, 2026 20:48
@jtmcdole jtmcdole added CICD Run CI/CD and removed CICD Run CI/CD labels Mar 24, 2026
Copy link
Contributor

@ievdokdm ievdokdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jtmcdole jtmcdole added the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 24, 2026
@auto-submit auto-submit bot merged commit ccfc427 into main Mar 24, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App. CICD Run CI/CD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants